Skip to content

Allow subclassing of SQLAlchemyObjectType without major API changes #51

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jul 27, 2017

Conversation

dfee
Copy link
Member

@dfee dfee commented Jun 2, 2017

This PR allows wholesale re-implementation of SQLAlchemyObjectType by revising the registry to ensure the classes are of type SQLAlchemyObjectTypeMeta, or subclassing SQLAlchemyObjectType (or a subclass thereof) by providing abstract = True on the subclass's Meta.

The docs have been updated to reflect this.

i.e.:

from graphene_sqlalchemy import SQLAlchemyObjectType

class ActiveSQLAlchemyObjectType(SQLAlchemyObjectType):
    class Meta:
        abstract = True

    @classmethod
    def get_node(cls, id, context, info):
        return cls.get_query(context).\
            filter(and_(
                cls._meta.model.deleted_at==None,
                cls._meta.model.id==id,
            )).\
            first()

class User(ActiveSQLAlchemyObjectType):
    class Meta:
        model = UserModel

class Query(graphene.ObjectType):
    users = graphene.List(User)

    def resolve_users(self, args, context, info):
        query = User.get_query(context) # SQLAlchemy query
        return query.all()

schema = graphene.Schema(query=Query)

@coveralls
Copy link

coveralls commented Jun 2, 2017

Coverage Status

Coverage increased (+0.1%) to 91.235% when pulling 0cd2ccf on dfee:master into 030e5e1 on graphql-python:master.

@dfee
Copy link
Member Author

dfee commented Jun 2, 2017

The pypy tests failed for some reason. Perhaps @syrusakbary can help me understand where the bug is (it looks like it's in your config script??…)

@syrusakbary
Copy link
Member

The error is just in the python lint:

graphene_sqlalchemy/registry.py:9:9: F401 '.types.SQLAlchemyObjectType' imported but unused

@coveralls
Copy link

coveralls commented Jun 6, 2017

Coverage Status

Coverage increased (+0.2%) to 91.27% when pulling 37d4331 on dfee:master into 030e5e1 on graphql-python:master.

@dfee
Copy link
Member Author

dfee commented Jun 6, 2017

Awesome. Thanks for the heads up. Revised!

@dfee
Copy link
Member Author

dfee commented Jun 20, 2017

@syrusakbary do you think this PR is viable?

@syrusakbary
Copy link
Member

syrusakbary commented Jul 27, 2017

Yup!

In Graphene 2.0 abstract types are available by default, and the Metaclass subclassing strategy is no longer necessary. However I'm going to merge and resolve the conflicts with 2.0 later :)

@syrusakbary syrusakbary merged commit d3f8e08 into graphql-python:master Jul 27, 2017
@tylfin
Copy link

tylfin commented Aug 9, 2017

When will this be put on pypi?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants